Skip to content

Allow unspecified alias destinations #254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

chuksys
Copy link
Contributor

@chuksys chuksys commented Apr 17, 2025

Description

This PR pulls up the network graph to make it simpler to run node lookups using public keys and aliases. This new node lookup approach is used to allow unspecified aliases in activity destinations by confirming if node(s) with unspecified alias exists in the graph.

Changes

  1. Added Graph struct to simln-lib to enable simpler node lookups.
  2. Added get_graph function to LightningNode trait and implemented it in the different LN implementations.
  3. Refactored activities validation to run lookup against Graph directly.
  4. Enabled mapping of duplicate aliases in alias_node_map and nodes_by_alias.

This PR closes #227

Copy link
Contributor Author

@chuksys chuksys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropped some notes. Open to suggestions.

@chuksys chuksys marked this pull request as ready for review April 22, 2025 01:25
Copy link
Contributor

@carlaKC carlaKC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Promising start, but I think that this PR has scope crept a bit. I'd really like to keep this down to just the case where we have a destination alias that isn't in our configured set of nodes. Feel free to squash and restructure as suggested, we can start with fixups when the PR's structure is in good shape.

Also think that we need to look more carefully at the design of the graph function in the LightningNode trait and move some of the more custom behavior into our validation function.

@chuksys
Copy link
Contributor Author

chuksys commented Apr 23, 2025

Promising start, but I think that this PR has scope crept a bit. I'd really like to keep this down to just the case where we have a destination alias that isn't in our configured set of nodes. Feel free to squash and restructure as suggested, we can start with fixups when the PR's structure is in good shape.

Also think that we need to look more carefully at the design of the graph function in the LightningNode trait and move some of the more custom behavior into our validation function.

Thanks for the feedback. Will sort this out asap!

@chuksys chuksys force-pushed the allow-unspecified-alias-destinations branch from 3b865f6 to 9fe6a15 Compare April 30, 2025 19:42
@chuksys chuksys requested a review from carlaKC April 30, 2025 20:41
Copy link
Contributor

@carlaKC carlaKC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving in the right direction!

@chuksys chuksys force-pushed the allow-unspecified-alias-destinations branch from 9fe6a15 to bf4bc6f Compare May 5, 2025 18:54
@chuksys chuksys requested a review from carlaKC May 5, 2025 19:10
Copy link
Contributor

@carlaKC carlaKC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few cleanups to go, but functionality looking good

@chuksys chuksys requested a review from carlaKC May 6, 2025 16:02
Copy link
Contributor

@carlaKC carlaKC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK on LND for the following cases:

  • Unique alias in network
  • Duplicate alias in network

@carlaKC
Copy link
Contributor

carlaKC commented May 6, 2025

Thanks for the quick turnaround on those last comments! You can go ahead and squash the fixups 👌

@chuksys have you been able to test this on CLN/Eclair manually? If possible I'd like us to have tested this on all the impls we support before merge - adding new things to LightningNode often has some dragons lurking that testing irons out nicely.

Also just a headsup that we've got a few things in the merge queue while docker issues #260 are resolved. So this might end up needing a rebase.

@carlaKC carlaKC requested a review from f3r10 May 6, 2025 17:33
@chuksys chuksys force-pushed the allow-unspecified-alias-destinations branch from 852a0ee to 351d922 Compare May 6, 2025 18:07
@chuksys
Copy link
Contributor Author

chuksys commented May 6, 2025

Thanks for the quick turnaround on those last comments!

Thank you! I appreciate you taking the time to review it. Happy to contribute.

@chuksys have you been able to test this on CLN/Eclair manually? If possible I'd like us to have tested this on all the impls we support before merge - adding new things to LightningNode often has some dragons lurking that testing irons out nicely.

I have tested this on LND and Eclair using polar but haven't done so on CLN. I'll test on CLN as soon as possible.

Also just a headsup that we've got a few things in the merge queue while docker issues #260 are resolved. So this might end up needing a rebase.

Thanks for the headsup. Happy to rebase as need be.

Copy link
Collaborator

@f3r10 f3r10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!
I have just tested on cln

{
  "nodes": [
{ 
  "id": "alice",
  "address": "127.0.0.1:9737",
  "ca_cert": "/tmp/l1/regtest/ca.pem",
  "client_cert": "/tmp/l1/regtest/client.pem",
  "client_key": "/tmp/l1/regtest/client-key.pem"
}
  ],
  "activity": [
 {
      "source": "alice",
      "destination": "SLEEPYDEITY-02-3-gb5eef8a-modded",
      "interval_secs": 1,
      "amount_msat": 10,
      "count": 10
    }
  ]
}

The results are:

sim-ln-review on  HEAD (351d922) [!+?] via 🦀 v1.85.0 via ❄️  impure (nix-shell-env) took 14s
❯ cargo run --bin sim-cli -- -l debug -s sim_cln_2.json
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.13s
     Running `target/debug/sim-cli -l debug -s sim_cln_2.json`
2025-05-07T15:44:39.999Z WARN  [simln_lib] The provided alias does not match the one returned by the backend (alice != STRANGECALENDAR--gb5eef8a-modded).
2025-05-07T15:44:40.000Z INFO  [sim_cli::parsing] Connected to alice - Node ID: 038213430d67a9fb52dfb3e6a2450de3bed5a8d081052cdc6dd7dec03f41e7b390.
2025-05-07T15:44:40.000Z INFO  [simln_lib] Running the simulation forever.
2025-05-07T15:44:40.002Z INFO  [simln_lib] Simulation is running on regtest.
2025-05-07T15:44:40.002Z INFO  [simln_lib] Simulating 1 activity on 1 nodes.
2025-05-07T15:44:40.002Z DEBUG [simln_lib] Setting up simulator data collection.
2025-05-07T15:44:40.002Z DEBUG [simln_lib] Simulator data collection set up.
2025-05-07T15:44:40.002Z DEBUG [simln_lib] Starting simulation results producer.
2025-05-07T15:44:40.002Z DEBUG [simln_lib] Starting results logger.
2025-05-07T15:44:40.002Z INFO  [simln_lib] Summary of results will be reported every 60s.
2025-05-07T15:44:40.002Z DEBUG [simln_lib] Starting simulation results consumer.
2025-05-07T15:44:40.002Z DEBUG [simln_lib] Starting events consumer for alice(038213...e7b390).
2025-05-07T15:44:40.002Z INFO  [simln_lib] Starting activity producer for alice(038213...e7b390): static payment of 10 to SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce) every 1s.
2025-05-07T15:44:40.002Z DEBUG [simln_lib] First payment for alice(038213...e7b390) in 1s.
2025-05-07T15:44:41.003Z DEBUG [simln_lib] Generated payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): 10 msat.
2025-05-07T15:44:41.004Z DEBUG [simln_lib] Next payment for alice(038213...e7b390) in 1s.
2025-05-07T15:44:42.004Z DEBUG [simln_lib] Generated payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): 10 msat.
2025-05-07T15:44:42.005Z DEBUG [simln_lib] Next payment for alice(038213...e7b390) in 1s.
2025-05-07T15:44:42.008Z DEBUG [simln_lib] Send payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): (f04acd71bb80bb8ec572c9300e7bcc130753ce4c282da045b1b475a65c2cdbef).
2025-05-07T15:44:43.005Z DEBUG [simln_lib] Generated payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): 10 msat.
2025-05-07T15:44:43.005Z DEBUG [simln_lib] Next payment for alice(038213...e7b390) in 1s.
2025-05-07T15:44:43.297Z DEBUG [simln_lib] Send payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): (cebdaebeb285504b65bd17cabf32997279804ee02eeba29c96028ef4a6c2b341).
2025-05-07T15:44:43.297Z DEBUG [simln_lib] Tracking payment outcome for: f04acd71bb80bb8ec572c9300e7bcc130753ce4c282da045b1b475a65c2cdbef.
2025-05-07T15:44:43.802Z DEBUG [simln_lib] Track payment f04acd71bb80bb8ec572c9300e7bcc130753ce4c282da045b1b475a65c2cdbef result: Success.
2025-05-07T15:44:44.006Z DEBUG [simln_lib] Generated payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): 10 msat.
2025-05-07T15:44:44.006Z DEBUG [simln_lib] Next payment for alice(038213...e7b390) in 1s.
2025-05-07T15:44:44.792Z DEBUG [simln_lib] Send payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): (05fd981e3a62a7eaea57a859c9969538e9b60ac5f9c5161f6d73ccdb7d14b040).
2025-05-07T15:44:44.793Z DEBUG [simln_lib] Tracking payment outcome for: cebdaebeb285504b65bd17cabf32997279804ee02eeba29c96028ef4a6c2b341.
2025-05-07T15:44:45.007Z DEBUG [simln_lib] Generated payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): 10 msat.
2025-05-07T15:44:45.007Z DEBUG [simln_lib] Next payment for alice(038213...e7b390) in 1s.
2025-05-07T15:44:45.298Z DEBUG [simln_lib] Track payment cebdaebeb285504b65bd17cabf32997279804ee02eeba29c96028ef4a6c2b341 result: Success.
2025-05-07T15:44:46.008Z DEBUG [simln_lib] Generated payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): 10 msat.
2025-05-07T15:44:46.288Z DEBUG [simln_lib] Send payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): (d4b2c2cf53f86d4f3df20e5373aa34052592bbc8ec593db056510d24735a1b52).
2025-05-07T15:44:46.289Z DEBUG [simln_lib] Next payment for alice(038213...e7b390) in 1s.
2025-05-07T15:44:46.289Z DEBUG [simln_lib] Tracking payment outcome for: 05fd981e3a62a7eaea57a859c9969538e9b60ac5f9c5161f6d73ccdb7d14b040.
2025-05-07T15:44:46.794Z DEBUG [simln_lib] Track payment 05fd981e3a62a7eaea57a859c9969538e9b60ac5f9c5161f6d73ccdb7d14b040 result: Success.
2025-05-07T15:44:47.289Z DEBUG [simln_lib] Generated payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): 10 msat.
2025-05-07T15:44:47.775Z DEBUG [simln_lib] Send payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): (52dd66071dacf22a57c46ef741716ebacf8d24d1ec8f90f8458d51f8765bf87c).
2025-05-07T15:44:47.775Z DEBUG [simln_lib] Next payment for alice(038213...e7b390) in 1s.
2025-05-07T15:44:47.775Z DEBUG [simln_lib] Tracking payment outcome for: d4b2c2cf53f86d4f3df20e5373aa34052592bbc8ec593db056510d24735a1b52.
2025-05-07T15:44:48.280Z DEBUG [simln_lib] Track payment d4b2c2cf53f86d4f3df20e5373aa34052592bbc8ec593db056510d24735a1b52 result: Success.
2025-05-07T15:44:48.776Z DEBUG [simln_lib] Generated payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): 10 msat.
2025-05-07T15:44:49.263Z DEBUG [simln_lib] Send payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): (be4351c62d73ae98040ad14d664686c20502e89d5b59bd131bff772a8ed6571b).
2025-05-07T15:44:49.263Z DEBUG [simln_lib] Next payment for alice(038213...e7b390) in 1s.
2025-05-07T15:44:49.263Z DEBUG [simln_lib] Tracking payment outcome for: 52dd66071dacf22a57c46ef741716ebacf8d24d1ec8f90f8458d51f8765bf87c.
2025-05-07T15:44:49.768Z DEBUG [simln_lib] Track payment 52dd66071dacf22a57c46ef741716ebacf8d24d1ec8f90f8458d51f8765bf87c result: Success.
2025-05-07T15:44:50.264Z DEBUG [simln_lib] Generated payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): 10 msat.
2025-05-07T15:44:50.761Z DEBUG [simln_lib] Send payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): (89ba4a4efee43c9c8bd46da399b1539ba192380b444e812716eac6ca3f32ae3c).
2025-05-07T15:44:50.761Z DEBUG [simln_lib] Next payment for alice(038213...e7b390) in 1s.
2025-05-07T15:44:50.761Z DEBUG [simln_lib] Tracking payment outcome for: be4351c62d73ae98040ad14d664686c20502e89d5b59bd131bff772a8ed6571b.
2025-05-07T15:44:51.266Z DEBUG [simln_lib] Track payment be4351c62d73ae98040ad14d664686c20502e89d5b59bd131bff772a8ed6571b result: Success.
2025-05-07T15:44:51.763Z DEBUG [simln_lib] Generated payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): 10 msat.
2025-05-07T15:44:52.296Z DEBUG [simln_lib] Send payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): (ea72e717da21000176779acf2f2066003eacabf566490816d7988501a6ee4d17).
2025-05-07T15:44:52.296Z INFO  [simln_lib] Payment count has been met for alice(038213...e7b390): 10 payments. Stopping the activity.
2025-05-07T15:44:52.296Z DEBUG [simln_lib] Activity producer for alice(038213...e7b390) completed successfully.
2025-05-07T15:44:52.296Z INFO  [simln_lib] All producers finished. Shutting down.
2025-05-07T15:44:52.296Z DEBUG [simln_lib] Tracking payment outcome for: 89ba4a4efee43c9c8bd46da399b1539ba192380b444e812716eac6ca3f32ae3c.
2025-05-07T15:44:52.296Z ERROR [simln_lib] Track payment failed for 89ba4a4efee43c9c8bd46da399b1539ba192380b444e812716eac6ca3f32ae3c: Track payment error: Shutdown before tracking results.
2025-05-07T15:44:52.296Z DEBUG [simln_lib] Track payment result received a shutdown signal.
2025-05-07T15:44:52.296Z DEBUG [simln_lib] Simulation results producer exiting.
2025-05-07T15:44:52.296Z DEBUG [simln_lib] Exiting results logger.
2025-05-07T15:44:52.296Z DEBUG [simln_lib] Consume simulation result received shutdown signal.
2025-05-07T15:44:52.296Z DEBUG [simln_lib] Produce simulation results received shutdown signal.
2025-05-07T15:44:53.609Z DEBUG [simln_lib] Send payment: alice(038213...e7b390) -> SLEEPYDEITY-02-3-gb5eef8a-modded(02a010...8151ce): (898ea80d1a4d83ed9fc3e1b69cdd46c2da717b0063b1e7d6a786f60d1fc47d92).
2025-05-07T15:44:53.609Z DEBUG [simln_lib] Event consumer for node alice(038213...e7b390) completed successfully.
2025-05-07T15:44:53.609Z DEBUG [simln_lib] Tracking payment outcome for: ea72e717da21000176779acf2f2066003eacabf566490816d7988501a6ee4d17.
2025-05-07T15:44:53.609Z ERROR [simln_lib] Track payment failed for ea72e717da21000176779acf2f2066003eacabf566490816d7988501a6ee4d17: Track payment error: Shutdown before tracking results.
2025-05-07T15:44:53.609Z DEBUG [simln_lib] Track payment result received a shutdown signal.

@carlaKC
Copy link
Contributor

carlaKC commented May 7, 2025

Needs rebase! You're next in the merge queue after #242 so perhaps you can go ahead and rebase on that so that once it's in you're ready to roll!

@chuksys
Copy link
Contributor Author

chuksys commented May 8, 2025

Okay great! Will do. Thanks 👍

@chuksys chuksys force-pushed the allow-unspecified-alias-destinations branch from 351d922 to 8bc8c47 Compare May 8, 2025 18:14
@chuksys
Copy link
Contributor Author

chuksys commented May 8, 2025

Just rebased on main since #242 landed early.

@carlaKC
Copy link
Contributor

carlaKC commented May 9, 2025

Style fix can be squashed with 33d64f7 then this is ready to land 👍

chuksys added 2 commits May 9, 2025 16:14
Switches from an async closure to direct NetworkGraph usage for activity destination
node lookup. This improves efficiency and maintainability, and enables destination
aliases by validating against the graph and erroring on duplicates.
@chuksys chuksys force-pushed the allow-unspecified-alias-destinations branch from 8bc8c47 to 690c4b1 Compare May 9, 2025 15:17
@carlaKC carlaKC merged commit 55cc91a into bitcoin-dev-project:main May 9, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Allow Destination nodes for un-specified aliases
3 participants